feat: Migrate JS-sided locks to DatabaseQueue for transactions and executeBatch commands
#227
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
chrispader
changed the title
DatabaseQueue for transactions and executeBatch commands
| openDatabaseQueue(options.name) | ||
| } catch (error) { | ||
| throw NitroSQLiteError.fromError(error) | ||
| } |
There was a problem hiding this comment.
Bug: Resource leak in database open error handling
Resource leak in error handling: If HybridNitroSQLite.open() succeeds but openDatabaseQueue() throws an error (e.g., if the database queue is already open), the native database will remain open while the database queue is not created. This leaves the system in an inconsistent state where the native database is open but the queue tracking doesn't exist. The code should either close the native database in the error handler, or call openDatabaseQueue() before opening the native database.
There was a problem hiding this comment.
This is true, but has been the behavior previously as well. I will address having multiple simultaneous database connections in a follow-up PR
| } catch (error) { | ||
| throw NitroSQLiteError.fromError(error) | ||
| } | ||
| }, |
There was a problem hiding this comment.
Bug: Resource Leaks from Failed Close Blocking Reopen
Resource leak in error handling: If HybridNitroSQLite.close() throws an error, closeDatabaseQueue() is never called, leaving the database queue entry in the databaseQueues map. This prevents the database from being reopened later since openDatabaseQueue() will throw an error saying the database is already open. The code should ensure closeDatabaseQueue() is called even if closing the native database fails, possibly by reversing the order or using a try-finally pattern.
Changes the current JS lock implementation in favor of a
DatabaseQueue, which is used for both transactions and batch executionsNote
Replace JS lock mechanism with a DatabaseQueue that serializes transactions and executeBatch operations, integrate it with session lifecycle, and update tests accordingly.
package/src/DatabaseQueue.tswithopenDatabaseQueue/closeDatabaseQueue,queueOperationAsync,startOperationSync, and per-DBqueue/inProgresstracking.transactionto run viaDatabaseQueue, add finalized-state guards, wrap errors withNitroSQLiteError, support optional exclusive begin, and return genericResult.executeBatch/executeBatchAsyncthroughDatabaseQueue; enforcethrowIfDatabaseIsNotOpen.open/closetoopenDatabaseQueue/closeDatabaseQueue; remove legacy lock usage.NitroSQLiteError; updateNitroSQLiteConnection.transactionsignature to be generic and return the callback result.example/src/tests/unit/specs/DatabaseQueue.spec.tsto verify queuing and error propagation for mixed operations.isNitroSQLiteError, adjust paths, and rely onresetTestDb(); exposetestDbQueue/TEST_DB_NAMEfor assertions.../packageinexample/tsconfig.json.Written by Cursor Bugbot for commit 9bcae9c. This will update automatically on new commits. Configure here.